Radiology report exports #203

jeremyestein · 2024-01-03T19:08:08Z

Define an API call in pixl ehr that triggers:

extract all EHR records from the PIXL DB that match the specified extract (as defined by project name and extract datetime)
dump to a single parquet file in the mounted export dir

For a later PR:

Adding a command to the CLI to call this API
Add to system test (depends on CLI call) - (add cli call after pixl populate, but how to wait until done processing)

Fixes #161

of code

monkeypatch-able class attribute.

pixl_ehr/src/pixl_ehr/main.py

Enable deferred annotations everywhere and fix annotations and comments (#206) * Defer annotations in all files and fix some pre-existing comment quirks * Applied ruff fixes with and without `--unsafe-fixes` flag * Some manual fixes

actual change] Merge branch 'main' into jeremy/issue-161-radiology

export, but some fields are missing.

pixl_core/src/core/omop.py

versions.

parquet extract

pixl_ehr/src/pixl_ehr/_databases.py

pixl_ehr/src/pixl_ehr/main.py

Should fix failing CI, where the `tests-ehr-api` Docker was failing to launch.

pixl_ehr/src/pixl_ehr/main.py

Allows exporting radiology reports for specific project only

return value anyway.

extract to parquet. Need different accession numbers in test messages to avoid them getting de-duped. Test code was only fetching first row of query so above problem was harder to detect.

stefpiatek

Looking good, only blocking change is adding in a new endpoint to the hashing API, worth discussing that and then some other thoughts

stefpiatek · 2024-01-11T12:07:07Z

hasher/src/hasher/endpoints.py

+@router.get(
+    "/hash-image-identifier",
+    summary="Produce secure hash appropriate for an image identifier "
+    "(MRN + accession num concatenated)",
+)
+async def hash_image_identifier(message: str) -> Response:
+    return Response(content=generate_hash(message, 64), media_type="application/text")


Oh this surprises me. Why are we making a seperate endpoint that has the same implementation as hash?

Mainly I was following the existing pattern. But do the different types of input even need different hashing methods? I'm not sure what the requirements are here.

Yeah I think the previous is a bit silly too. I'd remove and just use /hash. I assume it was planned to do something like restricting the size of it like they did with accession number, but then that changed and they didn't remove the superflous extra endpoint

Have removed the new endpoint. Shall we save the consolidation of the other endpoints for another PR?

Maybe make a new issue so we don't forget / to discuss further?

pixl_core/src/core/omop.py

pixl_ehr/pyproject.toml

pixl_ehr/src/pixl_ehr/main.py

pixl_ehr/tests/test_processing.py

postgres/pixl-db_init.sh

pixl_core/src/core/omop.py

stefpiatek

Looks good, couple of smaller changes but happy to merge once you've resolved them

cli/tests/conftest.py

pixl_ehr/src/pixl_ehr/_processing.py

Co-authored-by: Stef Piatek <s.piatek@ucl.ac.uk>

jeremyestein added 7 commits January 3, 2024 15:53

WIP - refactor in preparation for Radiology Extract; plus rough sketch

9f90e29

of code

WIP - tidy up

a860920

Pre-commit fixes

d6501f8

Fix circular import problem when running test

b54991a

Fix tests previously calling ParquetExport. Make the root dir a

89a75aa

monkeypatch-able class attribute.

pre-commmit fixes

0a1e80b

Fix broken import line

2af5f9a

stefpiatek reviewed Jan 4, 2024

View reviewed changes

pixl_ehr/src/pixl_ehr/main.py Outdated Show resolved Hide resolved

stefpiatek reviewed Jan 4, 2024

View reviewed changes

pixl_ehr/src/pixl_ehr/main.py Outdated Show resolved Hide resolved

jeremyestein added 8 commits January 4, 2024 12:33

Oops think I needed a real merge commit to mark it as resolved [no

a2199fc

actual change] Merge branch 'main' into jeremy/issue-161-radiology

Merge branch 'main' into jeremy/issue-161-radiology

1e4f811

Fixup merge

4d0d8e7

WIP - try to define the PIXL DB query and conversion of data for parquet

0c2addb

export, but some fields are missing.

Fix some pre-commit issues

1ed9a70

Further sketching

814bed7

Add pandas dep

fe76b68

stefpiatek reviewed Jan 5, 2024

View reviewed changes

pixl_core/src/core/omop.py Outdated Show resolved Hide resolved

pixl_core/src/core/omop.py Outdated Show resolved Hide resolved

pixl_core/src/core/omop.py Outdated Show resolved Hide resolved

pixl_core/src/core/omop.py Outdated Show resolved Hide resolved

jeremyestein added 3 commits January 5, 2024 16:48

Add image identifier to ehr_* tables and fill in the plain and hashed

fbb747d

versions.

Add procedure_occurrence_id to the DB as well, since we need it in the

b22ebe2

parquet extract

Export to parquet and more pre-commit fixes

06c7147

jeremyestein commented Jan 5, 2024

View reviewed changes

pixl_ehr/src/pixl_ehr/_databases.py Outdated Show resolved Hide resolved

milanmlft reviewed Jan 8, 2024

View reviewed changes

pixl_ehr/src/pixl_ehr/main.py Outdated Show resolved Hide resolved

milanmlft and others added 7 commits January 8, 2024 13:48

Always import datetime to prevent pydantic error

d479f5b

Should fix failing CI, where the `tests-ehr-api` Docker was failing to launch.

Procedure occurrence ID should be an int

14345b1

Add missing columns in test

07fb60f

Symlink radiology export

c32dd88

Call the new code from the test. Need pyarrow in ehr-api

ee23522

Add column names to parquet file

e9413cd

Merge branch 'main' into jeremy/issue-161-radiology

e52f023

jeremyestein added 3 commits January 9, 2024 16:13

Raise exception rather than assert

98a422b

Using list instead of Iterable makes error go away

acbff17

Return full path so can check during testing

90ddf0a

jeremyestein commented Jan 9, 2024

View reviewed changes

pixl_ehr/src/pixl_ehr/main.py Outdated Show resolved Hide resolved

milanmlft and others added 7 commits January 9, 2024 19:10

Add project_name to PIXL DB table

9895aac

Allows exporting radiology reports for specific project only

Test radiology exports

0eb3837

Fix mypy warning re return type of decorated function. We don't need the

9802dae

return value anyway.

Match on project name *and* extract timestamp when defining what to

adbee69

extract to parquet. Need different accession numbers in test messages to avoid them getting de-duped. Test code was only fetching first row of query so above problem was harder to detect.

Check parquet file doesn't reveal unhashed identifiers

e09d4b7

method should not start with test_ if not a test

dcb5118

Fix some comments

0c23188

jeremyestein marked this pull request as ready for review January 11, 2024 12:02

jeremyestein requested a review from stefpiatek January 11, 2024 12:13

stefpiatek reviewed Jan 11, 2024

View reviewed changes

jeremyestein and others added 4 commits January 11, 2024 14:50

Comment fixes

ead9848

Put query->DataFrame conversion code in the same place

d8b1339

Clarify comment

161b158

Test that cogstack has been run

06bb90c

milanmlft reviewed Jan 11, 2024

View reviewed changes

pixl_core/src/core/omop.py Outdated Show resolved Hide resolved

jeremyestein added 3 commits January 11, 2024 16:59

Rename file as it's no longer just about OMOP

728ac08

Merge branch 'main' into jeremy/issue-161-radiology

6eabe11

Remove superfluous hashing API endpoint; just use the generic one.

41c2371

stefpiatek approved these changes Jan 12, 2024

View reviewed changes

cli/tests/conftest.py Outdated Show resolved Hide resolved

pixl_ehr/src/pixl_ehr/_processing.py Outdated Show resolved Hide resolved

milanmlft and others added 4 commits January 12, 2024 10:11

Merge branch 'main' into jeremy/issue-161-radiology

4f4d89a

Update project_name and extract_datetime to be non-optional

0cdfda8

Update docstrings for _omop_files()

8fc8f50

Co-authored-by: Stef Piatek <s.piatek@ucl.ac.uk>

Format docstring

ab6b200

jeremyestein merged commit b51943f into main Jan 12, 2024
8 checks passed

jeremyestein deleted the jeremy/issue-161-radiology branch January 12, 2024 10:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Radiology report exports #203

Radiology report exports #203

jeremyestein commented Jan 3, 2024 •

edited by milanmlft

Loading

stefpiatek left a comment

stefpiatek Jan 11, 2024

jeremyestein Jan 11, 2024

stefpiatek Jan 11, 2024 •

edited

Loading

jeremyestein Jan 11, 2024

milanmlft Jan 12, 2024

stefpiatek left a comment

Radiology report exports #203

Radiology report exports #203

Conversation

jeremyestein commented Jan 3, 2024 • edited by milanmlft Loading

stefpiatek left a comment

Choose a reason for hiding this comment

stefpiatek Jan 11, 2024

Choose a reason for hiding this comment

jeremyestein Jan 11, 2024

Choose a reason for hiding this comment

stefpiatek Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

jeremyestein Jan 11, 2024

Choose a reason for hiding this comment

milanmlft Jan 12, 2024

Choose a reason for hiding this comment

stefpiatek left a comment

Choose a reason for hiding this comment

jeremyestein commented Jan 3, 2024 •

edited by milanmlft

Loading

stefpiatek Jan 11, 2024 •

edited

Loading